Skip to content

Conversation

@Lil-Ran
Copy link

@Lil-Ran Lil-Ran commented Jan 29, 2026

This is the first implementation of my suggested changes in the issue.

Some proposals are open for discussion, though the PR already includes test cases.

I would appreciate any feedback or suggestions on these proposed changes.

@python-cla-bot
Copy link

python-cla-bot bot commented Jan 29, 2026

All commit authors signed the Contributor License Agreement.

CLA signed


def test_invalid_specifier_type_error_message(self):
for value in [12j, 12, 12.0, "12"]:
for bad_spec, repr in [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: repr is a built-in.

Suggested change
for bad_spec, repr in [
for bad_spec, repr_str in [

]:
with self.subTest(value=value, bad_spec=bad_spec):
err = re.escape("Unknown format code "
f"{repr} for object of type "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
f"{repr} for object of type "
f"{repr_str} for object of type "

@serhiy-storchaka serhiy-storchaka self-requested a review February 4, 2026 08:21
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable.

if (actual_format_spec != NULL) {
PyErr_Format(PyExc_ValueError,
"Invalid format specifier '%U' for object of type '%.200s'",
"Invalid format specifier %R for object of type '%.200s'",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we are here, we can use %T.

}
}

if (end-pos) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are already some checks above. I think that it is better to modify them. Your code emits weird error message for formats ,10, or _10_.

}
else if (Py_UNICODE_ISPRINTABLE(conversion)) {
PyErr_Format(PyExc_ValueError,
"Unknown conversion specifier '%c' (U+%04X)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such code is now repeated 3 times. Maybe add a helper function? For example, a function that formats "'%c' (U+%04X)" (with special cases).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants